fix: add validatorMinStake to getEpochInfo#146
Open
MuncleUscles wants to merge 6 commits intomainfrom
Open
Conversation
Add Bradbury testnet as a new network option with v0.6 ConsensusMain and ConsensusData ABIs, reusing the existing Staking ABI. Bradbury is the developer-facing testnet for deploying intelligent contracts with LLM validation.
- decodeTransaction: normalize initialRotations/txCalldata field names from Bradbury ABI (V06) which differ from Asimov's numOfInitialValidators/txData - Update testnet RPC URLs to GenLayer RPC nodes - Remove WebSocket URLs from testnet chains (not available) - Add unit tests for Bradbury field normalization in decodeTransaction - Add smoke tests for transaction decoding, gen_call, and account balance - Switch smoke tests from raw viem clients to genlayer-js createClient to avoid id:0 rejection by GenLayer RPC
The staking wizard reads epochInfo.validatorMinStakeRaw and epochInfo.validatorMinStake but getEpochInfo never fetched them, causing "Minimum stake required: undefined" and then "Cannot mix BigInt and other types" when comparing undefined + BigInt. - Add validatorMinStake() view function to STAKING_ABI (public state variable getter was missing) - Fetch validatorMinStake in getEpochInfo - Add validatorMinStakeRaw (bigint) and validatorMinStake (string) to EpochInfo type
📝 WalkthroughWalkthroughThis PR adds validator minimum stake querying to the staking ABI, normalizes transaction decoding fields, updates testnet RPC URLs with WebSocket removal, and extends the EpochInfo type while updating corresponding test coverage and Vitest configuration. Changes
Sequence DiagramsequenceDiagram
participant Client
participant getEpochInfo as getEpochInfo Action
participant Contract as Smart Contract
participant Formatter as formatEther
Client->>getEpochInfo: Request epoch info
getEpochInfo->>Contract: contract.read.validatorMinStake()
Contract-->>getEpochInfo: validatorMinStakeRaw (bigint)
getEpochInfo->>Formatter: formatEther(validatorMinStakeRaw)
Formatter-->>getEpochInfo: Formatted stake (string)
getEpochInfo-->>Client: EpochInfo with validatorMinStake fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/chains/testnetAsimov.ts`:
- Around line 6-7: The constant TESTNET_JSON_RPC_URL is set to a plaintext
http:// IP which weakens transport security; update the default to a TLS
endpoint (use an https:// hostname rather than http://IP) and allow overriding
via environment (e.g., read from process.env.TESTNET_JSON_RPC_URL fallback to
the new https URL); locate and change the TESTNET_JSON_RPC_URL definition in
testnetAsimov.ts and ensure any callers expect an HTTPS RPC URL.
In `@src/chains/testnetBradbury.ts`:
- Around line 5-6: The default TESTNET_JSON_RPC_URL constant is using an
insecure "http://" scheme; update the TESTNET_JSON_RPC_URL value to use
"https://" (e.g., "https://34.91.102.53:9151") so the SDK defaults use TLS.
Locate the TESTNET_JSON_RPC_URL constant in src/chains/testnetBradbury.ts and
replace the plaintext HTTP URL with the HTTPS URL, ensuring any downstream code
that relies on this constant continues to function with the secure endpoint.
In `@tests/smoke.test.ts`:
- Around line 166-170: The test labeled for transaction decoding currently only
calls createClient({chain}) and client.getBlockNumber(), so it never exercises
the getTransaction decoding path; modify the test body to fetch a recent
finalized transaction using the client.getBlock or client.getTransaction APIs:
obtain a recent block via client.getBlockNumber() then
client.getBlock(blockNumber) (or client.getBlockWithTransactions) to retrieve at
least one transaction hash and call client.getTransaction(txHash) to ensure
decoding runs, and add assertions that the returned transaction object is
defined and contains expected fields (e.g., hash and to/from) while keeping the
same TIMEOUT and test name.
- Around line 179-189: The current catch for client.request({ method: "gen_call"
}) only asserts absence of "method not found" but still allows network/transport
failures to pass; update the catch in tests/smoke.test.ts to detect
transport-level errors and fail the test for those (e.g., inspect e.code for
common codes like "ECONNREFUSED"/"ENETUNREACH" or e.message containing
"connect"/"network"/"timed out") and rethrow or call expect.fail for transport
errors, otherwise continue to assert the error message does not contain "method
not found"/"method_not_found" so genuine RPC errors still succeed the
connectivity smoke check.
In `@tests/transactions.test.ts`:
- Around line 4-6: Update the import paths in the test so they use the project
path aliases instead of relative paths: replace imports of decodeTransaction and
simplifyTransactionReceipt from "../src/transactions/decoders" with
"@/transactions/decoders", replace localnet from "../src/chains/localnet" with
"@/chains/localnet", and replace the GenLayerRawTransaction type import from
"../src/types/transactions" with "@/types/transactions" (use @@/tests/* only for
imports that come from the tests directory). Ensure the imported symbols
(decodeTransaction, simplifyTransactionReceipt, localnet,
GenLayerRawTransaction) are unchanged and the file still compiles with the alias
resolver.
In `@tsconfig.vitest-temp.json`:
- Line 31: The tsBuildInfoFile value in tsconfig.vitest-temp.json is an
absolute, machine-specific path; update the tsBuildInfoFile entry (the
"tsBuildInfoFile" key) to use a repo-relative path (e.g.
"node_modules/vitest/dist/chunks/tsconfig.tmp.tsbuildinfo" or
"./node_modules/…") so it no longer contains a /Users/... absolute path and is
portable across environments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 54774110-b7d8-4c47-b9a9-61a6a692da68
📒 Files selected for processing (9)
src/abi/staking.tssrc/chains/testnetAsimov.tssrc/chains/testnetBradbury.tssrc/staking/actions.tssrc/transactions/decoders.tssrc/types/staking.tstests/smoke.test.tstests/transactions.test.tstsconfig.vitest-temp.json
Comment on lines
+6
to
+7
| const TESTNET_JSON_RPC_URL = "http://34.12.136.220:9151"; | ||
| // WebSocket not available on testnet GenLayer RPC nodes |
There was a problem hiding this comment.
Use a TLS RPC endpoint for default chain config.
Line 6 hardcodes a plaintext http:// IP endpoint. This downgrades transport guarantees and makes RPC traffic easier to intercept/tamper with on untrusted networks. Prefer a stable https:// hostname endpoint as the default.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/chains/testnetAsimov.ts` around lines 6 - 7, The constant
TESTNET_JSON_RPC_URL is set to a plaintext http:// IP which weakens transport
security; update the default to a TLS endpoint (use an https:// hostname rather
than http://IP) and allow overriding via environment (e.g., read from
process.env.TESTNET_JSON_RPC_URL fallback to the new https URL); locate and
change the TESTNET_JSON_RPC_URL definition in testnetAsimov.ts and ensure any
callers expect an HTTPS RPC URL.
Comment on lines
+5
to
+6
| const TESTNET_JSON_RPC_URL = "http://34.91.102.53:9151"; | ||
| // WebSocket not available on testnet GenLayer RPC nodes |
There was a problem hiding this comment.
Default Bradbury RPC should use HTTPS, not plaintext HTTP.
Line 5 sets a non-TLS IP endpoint as default. For SDK defaults, prefer a secure https:// host endpoint to avoid transport integrity/privacy degradation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/chains/testnetBradbury.ts` around lines 5 - 6, The default
TESTNET_JSON_RPC_URL constant is using an insecure "http://" scheme; update the
TESTNET_JSON_RPC_URL value to use "https://" (e.g., "https://34.91.102.53:9151")
so the SDK defaults use TLS. Locate the TESTNET_JSON_RPC_URL constant in
src/chains/testnetBradbury.ts and replace the plaintext HTTP URL with the HTTPS
URL, ensuring any downstream code that relies on this constant continues to
function with the secure endpoint.
Comment on lines
+166
to
+170
| it("getTransaction should decode without crashing on a recent finalized tx", async () => { | ||
| const client = createClient({chain}); | ||
| const blockNumber = await client.getBlockNumber(); | ||
| expect(blockNumber).toBeGreaterThan(0n); | ||
| }, TIMEOUT); |
There was a problem hiding this comment.
Transaction decoding test does not exercise decoding path.
Line 166 states this validates getTransaction decode behavior, but the test only checks getBlockNumber(). This can miss the exact regression this block is intended to catch.
Suggested minimal correction
-describe(`Testnet ${name} - Transaction Decoding`, () => {
- it("getTransaction should decode without crashing on a recent finalized tx", async () => {
+describe(`Testnet ${name} - Transaction Decoding`, () => {
+ it("should fetch and decode at least one recent transaction", async () => {
const client = createClient({chain});
- const blockNumber = await client.getBlockNumber();
- expect(blockNumber).toBeGreaterThan(0n);
+ // Exercise transaction retrieval/decoding path here (not just block height).
+ // Example: fetch a recent block with txs, then call `getTransaction` for one hash.
}, TIMEOUT);
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/smoke.test.ts` around lines 166 - 170, The test labeled for transaction
decoding currently only calls createClient({chain}) and client.getBlockNumber(),
so it never exercises the getTransaction decoding path; modify the test body to
fetch a recent finalized transaction using the client.getBlock or
client.getTransaction APIs: obtain a recent block via client.getBlockNumber()
then client.getBlock(blockNumber) (or client.getBlockWithTransactions) to
retrieve at least one transaction hash and call client.getTransaction(txHash) to
ensure decoding runs, and add assertions that the returned transaction object is
defined and contains expected fields (e.g., hash and to/from) while keeping the
same TIMEOUT and test name.
Comment on lines
+179
to
+189
| try { | ||
| await client.request({ | ||
| method: "gen_call" as any, | ||
| params: [{ type: "read", to: "0x0000000000000000000000000000000000000000", from: "0x0000000000000000000000000000000000000000", data: "0x" }], | ||
| }); | ||
| } catch (e: any) { | ||
| // We expect an RPC error (invalid contract, etc.), NOT a "method not found" error | ||
| const msg = (e.message || e.details || "").toLowerCase(); | ||
| expect(msg).not.toContain("method not found"); | ||
| expect(msg).not.toContain("method_not_found"); | ||
| } |
There was a problem hiding this comment.
gen_call availability check is too permissive on transport failures.
If the RPC is unreachable, this can still pass as long as the error message doesn’t contain "method not found". Smoke connectivity should fail on network/transport errors.
Suggested tightening
try {
await client.request({
method: "gen_call" as any,
params: [{ type: "read", to: "0x0000000000000000000000000000000000000000", from: "0x0000000000000000000000000000000000000000", data: "0x" }],
});
} catch (e: any) {
const msg = (e.message || e.details || "").toLowerCase();
expect(msg).not.toContain("method not found");
expect(msg).not.toContain("method_not_found");
+ expect(msg).not.toContain("network");
+ expect(msg).not.toContain("fetch");
+ expect(msg).not.toContain("timeout");
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/smoke.test.ts` around lines 179 - 189, The current catch for
client.request({ method: "gen_call" }) only asserts absence of "method not
found" but still allows network/transport failures to pass; update the catch in
tests/smoke.test.ts to detect transport-level errors and fail the test for those
(e.g., inspect e.code for common codes like "ECONNREFUSED"/"ENETUNREACH" or
e.message containing "connect"/"network"/"timed out") and rethrow or call
expect.fail for transport errors, otherwise continue to assert the error message
does not contain "method not found"/"method_not_found" so genuine RPC errors
still succeed the connectivity smoke check.
Comment on lines
+4
to
+6
| import { decodeTransaction, simplifyTransactionReceipt } from "../src/transactions/decoders"; | ||
| import { localnet } from "../src/chains/localnet"; | ||
| import type { GenLayerRawTransaction } from "../src/types/transactions"; |
There was a problem hiding this comment.
Switch changed test imports to @/* aliases.
Line 4 and Line 6 import from ../src/... instead of @/....
Proposed fix
-import { decodeTransaction, simplifyTransactionReceipt } from "../src/transactions/decoders";
+import { decodeTransaction, simplifyTransactionReceipt } from "@/transactions/decoders";
import { localnet } from "../src/chains/localnet";
-import type { GenLayerRawTransaction } from "../src/types/transactions";
+import type { GenLayerRawTransaction } from "@/types/transactions";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { decodeTransaction, simplifyTransactionReceipt } from "../src/transactions/decoders"; | |
| import { localnet } from "../src/chains/localnet"; | |
| import type { GenLayerRawTransaction } from "../src/types/transactions"; | |
| import { decodeTransaction, simplifyTransactionReceipt } from "@/transactions/decoders"; | |
| import { localnet } from "../src/chains/localnet"; | |
| import type { GenLayerRawTransaction } from "@/types/transactions"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/transactions.test.ts` around lines 4 - 6, Update the import paths in
the test so they use the project path aliases instead of relative paths: replace
imports of decodeTransaction and simplifyTransactionReceipt from
"../src/transactions/decoders" with "@/transactions/decoders", replace localnet
from "../src/chains/localnet" with "@/chains/localnet", and replace the
GenLayerRawTransaction type import from "../src/types/transactions" with
"@/types/transactions" (use @@/tests/* only for imports that come from the tests
directory). Ensure the imported symbols (decodeTransaction,
simplifyTransactionReceipt, localnet, GenLayerRawTransaction) are unchanged and
the file still compiles with the alias resolver.
| "skipLibCheck": true, | ||
| "emitDeclarationOnly": false, | ||
| "incremental": true, | ||
| "tsBuildInfoFile": "/Users/edgars/Dev/genlayer-js/node_modules/vitest/dist/chunks/tsconfig.tmp.tsbuildinfo" |
There was a problem hiding this comment.
Make tsBuildInfoFile repo-relative (not machine-specific).
Line 31 points to a local absolute path under /Users/..., which is not portable and can fail outside your workstation.
Proposed fix
- "tsBuildInfoFile": "/Users/edgars/Dev/genlayer-js/node_modules/vitest/dist/chunks/tsconfig.tmp.tsbuildinfo"
+ "tsBuildInfoFile": "./node_modules/.cache/vitest/tsconfig.tmp.tsbuildinfo"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "tsBuildInfoFile": "/Users/edgars/Dev/genlayer-js/node_modules/vitest/dist/chunks/tsconfig.tmp.tsbuildinfo" | |
| "tsBuildInfoFile": "./node_modules/.cache/vitest/tsconfig.tmp.tsbuildinfo" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tsconfig.vitest-temp.json` at line 31, The tsBuildInfoFile value in
tsconfig.vitest-temp.json is an absolute, machine-specific path; update the
tsBuildInfoFile entry (the "tsBuildInfoFile" key) to use a repo-relative path
(e.g. "node_modules/vitest/dist/chunks/tsconfig.tmp.tsbuildinfo" or
"./node_modules/…") so it no longer contains a /Users/... absolute path and is
portable across environments.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getEpochInfo()never fetchedvalidatorMinStakefrom the staking contract, returningundefinedforvalidatorMinStakeRaw/validatorMinStakevalidatorMinStake()view function to STAKING_ABI (public state variable getter was missing)getEpochInfoand include inEpochInforeturn typeTest plan
npm run buildsucceedsvalidatorMinStake(matches on-chainuint256 public validatorMinStake)Summary by CodeRabbit
New Features
Updates
Refactor